I think this might be wrong? ZK_ARGS_LENGTH is 172 but this encodes 160 bytes (32*5 bytes)? Am I missing something?

Dug into this further, it is wrong, and it's not just a length mismatch. Walked through the full call chain and wrote a repro.

zkGameArgEncoder (dispute_games.go:265) is a w3 ABI tuple encoder, producing 5 × 32 = 160 bytes with per-field left-padding. The bytes flow through SetDisputeGameImpl.s.sol:75 into factory.setImplementation(gameType, impl, gameArgs) (DisputeGameFactory.sol:315-316), which stores them verbatim and appends them to every clone as CWIA immutable data (:210). ZKDisputeGame.sol:181-217 reads the packed layout at fixed byte offsets, and since LibGameArgs.sol:116 reverts with InvalidGameArgsLength on anything other than 172 bytes, any clone instantiated against this impl reverts on creation. Not silently broken, actively unusable.

The same tuple encoder works fine in upgrade/embedded/upgrade.go because OPCM re-abi.decodes and re-packs it on-chain via OPContractsManagerUtils.makeGameArgs (:455-467). The deployer path skips that step. The Cannon peer at dispute_games.go:189 uses gameargs.PackPermissionless(), which is the precedent to follow.

Also noticed the blob is missing three fields entirely: anchorStateRegistry, weth, and l2ChainId. Even if we switched to packed encoding without fixing that, we'd still be short, and those need to come from thisState.OpChainContracts and thisIntent.ID.

Wrote a quick repro: https://gist.github.com/wwared/75bf76f775a44410ade3fac80ce402d9

gh pr checkout 20219 --repo ethereum-optimism/optimism
curl -L https://gist.githubusercontent.com/wwared/75bf76f775a44410ade3fac80ce402d9/raw \
  -o op-deployer/pkg/deployer/pipeline/zk_gameargs_encoding_test.go
go test -run 'TestZKGameArgs_' -v ./op-deployer/pkg/deployer/pipeline/...

It encodes a known config via zkGameArgEncoder, strips the selector the same way dispute_games.go:157 does, and compares against the reference 172-byte packed layout. A PASS on TestZKGameArgs_DeployerEncoderProducesWrongLayout means the args are encoded incorrectly: each assertion pins one fact about the broken state (len == 160, verifier reads back as 0x000…0001111…1111 from the ABI pad, etc.). Once the fix lands the assertions flip, and at that point this should be replaced with a golden-byte positive-path test.

On why we didn't catch it earlier: the unit tests in dispute_games_test.go are all negative paths (…_ZeroImpl, …_NilParams, …_WrongDisputeGameType, …_NilChallengerBond, …_ZeroChallengerBond), which return before the encoder runs, so nothing inspects its output. TestEncodedUpgradeInputV2_GameArgsEncoding does pin the 160-byte ABI form byte-for-byte, but only for the upgrade path where OPCM re-packs. And the new smoke test in op-acceptance-tests/tests/proofs/zk/smoke_test.go routes through delegateCallWithSetCode on the OPCM impl (in sysgo/add_game_type.go), which is the upgrade path again. The new DeployAdditionalDisputeGames ZK branch has no coverage.

Fix-wise: swap the tuple encoder for a packed encoder matching OPContractsManagerUtils.makeGameArgs for ZK_DISPUTE_GAME (172 bytes, appending ASR/WETH/chainID), following the Cannon gameargs.PackPermissionless() pattern. Add a golden-byte positive-path test. And ideally a test that actually exercises the deployer path rather than only the upgrade flow.

Note: it's entirely possible Claude got some stuff wrong in the above comment, but at the very least it does seem like the game args on the ZKDisputeGame.sol contract are different from the ones in this PR (missing the last three):

You were right, fixed everything you flagged.

Swapped the w3 ABI tuple encoder for a proper ZKGameArgs.Pack() in the gameargs package (same pattern as PackPermissionless() for Cannon), now produces the correct 172 packed bytes. Also added the three missing fields (anchorStateRegistry, weth, l2ChainId) from thisState.OpChainContracts and thisIntent.ID, plus a test pinning every offset.

The end-to-end acceptance test for the deployer path is a follow-up, agreed it's missing but I will leave it as a separate issue.

This InitBond value isn't ever being read from this struct I think, is it? If I'm correct it's only being read from the DisputeGameConfig.InitBond field.

If it's not, we should either wire it up in the upgrade path or remove it completely.

I think it's confusing to have InitBond in multiple places, so we should only have it in either DisputeGameConfig or in ZKDisputeGameParams but not both.

nit/question: Here we use thisState.OpChainContracts.DelayedWethPermissionlessGameProxy (makes sense, ZK is permissionless), but in line 202 below, we use thisState.OpChainContracts.DelayedWethPermissionedGameProxy for both permissioned and permissionless cannon games in the deploy.

This question is not about a change in this PR (this behavior was already in op-deployer previously), and I think it is correct to use the permissionless proxy for ZK games, but I'm raising this inconsistency/difference so someone with more context around these contracts can double check (cc @stevennevins)

I'd recommend we add the following checks to this block as well:

  if game.ZKDisputeGame.MaxChallengeDuration == 0 {
      return fmt.Errorf("%w: MaxChallengeDuration must be > 0, chainId=%s", ErrZKDisputeGameMissingParams, c.ID)
  }
  if game.ZKDisputeGame.MaxProveDuration == 0 {
      return fmt.Errorf("%w: MaxProveDuration must be > 0, chainId=%s", ErrZKDisputeGameMissingParams, c.ID)
  }
  if game.ZKDisputeGame.ChallengerBond == nil || game.ZKDisputeGame.ChallengerBond.ToInt().Sign() <= 0 {
      return fmt.Errorf("%w: ChallengerBond must be set to a positive value, chainId=%s", ErrZKDisputeGameMissingParams, c.ID)
  }

The first two are new checks, the third one is checked at deployDisputeGame time I think, but it would be good to check it earlier at intent time too.

I don't think we have these checks on the smart contracts, so we should have them on the Go deployer side